V2 update/gas optimization #69
Conversation
There was a problem hiding this comment.
I feel like this type of cache gonna be very hard to manage and will spawn accidental bugs. From what I spotted with glance is autocompound doesn't reset position, meaning it won't notice new rewards until deposit/position update called, but I don't think bot should be allowed to call update (no check of sender there, but it looks like it should be admin method)
If I had to propose something:
What do you think about loading it at the start of every method that needs it and re-using for this method?
So for example you will have Strategy::load_osmosis_positions and it will return [(position_id, FullPositionBreakdown)] that you have to pass into all of the methods that can require FullPositionBreakdown. In the worst case you'll have to do some clones, but it should be cheaper than serialize/deserialize/store it completely and easier to manage
| // This is a cache to avoid querying the position information as this query is expensive | ||
| pub position_cache: Option<FullPositionBreakdown>, |
There was a problem hiding this comment.
This query is expensive and serializing/deserializing/saving in state this structure is also expensive(it's pretty huge), so if we do cache it between calls I don't think we should be sending all of it on every message, but only information that we absolutely need for this execution.
There was a problem hiding this comment.
Again, I'm NEVER saving the structure in state ! This field is just there to store the cache temporarily
There was a problem hiding this comment.
Again, I'm NEVER saving the structure in state ! This field is just there to store the cache temporarily
Ah, I see, thanks for clearing that up. Although I would prefer to keep it separately so it doesn't get stored accidentally and no time or gas wasted for serializing/deserializing
There was a problem hiding this comment.
If I make the storage structure private, it might solve the issue
There was a problem hiding this comment.
If I make the storage structure private, it might solve the issue
You can make wrapper around Strategy that will have this field(or other useful fields for other protocols) like
StrategyFetched{
strategy: Strategy,
osmosis_positions: HashSet<position_id, FullPositionBreakDown>,
// .. other useful info after fetching strategies
}
I'm not storing anything here ! It's just stored temporarily in the strategy object. The serialize/deserialize part is not that expensive either. |
This PR aims at adding a cache system for yield positions to avoid expensive queries.
We introduce 2 things :
This allows reducing gas price from 3.1M to 2.1M for an add-to-position operation